Skip to content

Conversation

@asb
Copy link
Contributor

@asb asb commented Jan 22, 2025

I'm not sure why the test is larger for RISC-V than other targets, but we saw this before with #111360.

The file is just over the current 60KB limit:

62772 /home/asb/llvm-project/build/stage2/tools/clang/test/Modules/Output/empty.modulemap.tmp/base.pcm

…RISC-V failure

I'm not sure why the test is larger for RISC-V than other targets, but
we saw this before with llvm#111360.

The file is just over the current 60KB limit:

```
62772 /home/asb/llvm-project/build/stage2/tools/clang/test/Modules/Output/empty.modulemap.tmp/base.pcm
```
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:modules C++20 modules and Clang Header Modules labels Jan 22, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 22, 2025

@llvm/pr-subscribers-clang-modules

Author: Alex Bradbury (asb)

Changes

I'm not sure why the test is larger for RISC-V than other targets, but we saw this before with #111360.

The file is just over the current 60KB limit:

62772 /home/asb/llvm-project/build/stage2/tools/clang/test/Modules/Output/empty.modulemap.tmp/base.pcm

Full diff: https://github.com/llvm/llvm-project/pull/123959.diff

1 Files Affected:

  • (modified) clang/test/Modules/empty.modulemap (+2-2)
diff --git a/clang/test/Modules/empty.modulemap b/clang/test/Modules/empty.modulemap
index f2d37c19d77bcc..8cad8b67b91155 100644
--- a/clang/test/Modules/empty.modulemap
+++ b/clang/test/Modules/empty.modulemap
@@ -13,8 +13,8 @@
 // The module file should be identical each time we produce it.
 // RUN: diff %t/base.pcm %t/check.pcm
 //
-// We expect an empty module to be less than 60KB (and at least 10K, for now).
+// We expect an empty module to be less than 70KB (and at least 10K, for now).
 // RUN: wc -c %t/base.pcm | FileCheck --check-prefix=CHECK-SIZE %s
-// CHECK-SIZE: {{(^|[^0-9])[1-5][0-9][0-9][0-9][0-9]($|[^0-9])}}
+// CHECK-SIZE: {{(^|[^0-9])[1-6][0-9][0-9][0-9][0-9]($|[^0-9])}}
 
 module empty { header "Inputs/empty.h" export * }

@llvmbot
Copy link
Member

llvmbot commented Jan 22, 2025

@llvm/pr-subscribers-clang

Author: Alex Bradbury (asb)

Changes

I'm not sure why the test is larger for RISC-V than other targets, but we saw this before with #111360.

The file is just over the current 60KB limit:

62772 /home/asb/llvm-project/build/stage2/tools/clang/test/Modules/Output/empty.modulemap.tmp/base.pcm

Full diff: https://github.com/llvm/llvm-project/pull/123959.diff

1 Files Affected:

  • (modified) clang/test/Modules/empty.modulemap (+2-2)
diff --git a/clang/test/Modules/empty.modulemap b/clang/test/Modules/empty.modulemap
index f2d37c19d77bcc..8cad8b67b91155 100644
--- a/clang/test/Modules/empty.modulemap
+++ b/clang/test/Modules/empty.modulemap
@@ -13,8 +13,8 @@
 // The module file should be identical each time we produce it.
 // RUN: diff %t/base.pcm %t/check.pcm
 //
-// We expect an empty module to be less than 60KB (and at least 10K, for now).
+// We expect an empty module to be less than 70KB (and at least 10K, for now).
 // RUN: wc -c %t/base.pcm | FileCheck --check-prefix=CHECK-SIZE %s
-// CHECK-SIZE: {{(^|[^0-9])[1-5][0-9][0-9][0-9][0-9]($|[^0-9])}}
+// CHECK-SIZE: {{(^|[^0-9])[1-6][0-9][0-9][0-9][0-9]($|[^0-9])}}
 
 module empty { header "Inputs/empty.h" export * }

Copy link
Member

@ChuanqiXu9 ChuanqiXu9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CC @Bigcheese

But I think the patch itself is good to go. Let's avoid breaking the CI.

@asb asb merged commit 1930635 into llvm:main Jan 23, 2025
11 checks passed
@asb
Copy link
Contributor Author

asb commented Jan 23, 2025

Thanks, I've gone ahead an merged. Any insight into why the module files may be larger on RISC-V vs other targets greatly appreciated - as I said in #111360 I'm not sure how to best inspect the module files.

@Bigcheese
Copy link
Contributor

It could be due to builtins, I believe we always serialize them, and that would be different between targets. Does RISC-V have a lot of them compared to other targets?

@asb
Copy link
Contributor Author

asb commented Jan 25, 2025

It could be due to builtins, I believe we always serialize them, and that would be different between targets. Does RISC-V have a lot of them compared to other targets?

That's almost certainly it then, thanks! We have a lot of vector builtins in particular.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants